Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixes for npm audit #4

Merged
merged 7 commits into from
Oct 24, 2022
Merged

fixes for npm audit #4

merged 7 commits into from
Oct 24, 2022

Conversation

briangough
Copy link

This PR makes some minor changes to our fork of the socket.io-client library

  • fix the ./bin/builder script so that it works with current node versions, it was using an fs.write call which stopped being supported in node 10 so we couldn't build the dist files any more
  • upgrade uglify-js as the previous version had a critical security vulnerability... we weren't using the minified version afaik but it adds noise when looking for real vulnerabilities in npm audit.
  • also moved to uglify-js to the dev dependencies (again this reduces the output with npm audit --omit dev).
  • since we don't use the node version in our fork I force upgraded xmlhttprequest and ws to versions without vulnerabilities to keep npm audit quiet.

Final result

$ npm audit --omit dev
found 0 vulnerabilities

Previous

$ npm audit --omit dev
# npm audit report

uglify-js  <=2.5.0
Severity: critical
Incorrect Handling of Non-Boolean Comparisons During Minification in uglify-js - https://github.com/advisories/GHSA-34r7-q49f-h37c
Regular Expression Denial of Service in uglify-js - https://github.com/advisories/GHSA-c9f4-xj24-8jqx
fix available via `npm audit fix --force`
Will install uglify-js@3.17.3, which is a breaking change
node_modules/uglify-js

ws  <=1.1.4
Severity: high
Denial of Service in ws - https://github.com/advisories/GHSA-5v72-xg48-5rpm
Remote Memory Disclosure in ws - https://github.com/advisories/GHSA-2mhh-w6q8-5hxw
DoS due to excessively large websocket message in ws - https://github.com/advisories/GHSA-6663-c963-2gqg
fix available via `npm audit fix --force`
Will install ws@8.9.0, which is a breaking change
node_modules/ws

xmlhttprequest  <1.7.0
Severity: high
Arbitrary Code Injection - https://github.com/advisories/GHSA-h4j5-c7cj-74xg
fix available via `npm audit fix --force`
Will install xmlhttprequest@1.8.0, which is outside the stated dependency range
node_modules/xmlhttprequest

3 vulnerabilities (2 high, 1 critical)

@briangough briangough self-assigned this Oct 21, 2022
@briangough briangough marked this pull request as ready for review October 21, 2022 15:02
@briangough briangough assigned das7pad and unassigned briangough Oct 21, 2022
das7pad
das7pad approved these changes Oct 21, 2022
View reviewed changes
Copy link
Member

@das7pad das7pad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One bonus comment.

package.json Outdated
"url": "https://github.com/LearnBoost/socket.io-client.git"
},
"dependencies": {
"active-x-obfuscator": "0.0.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bonus: It looks like this is a dev-dependency as well (there is only one reference in the builder script). WDYT about moving it over to the other category?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. good one. I've moved that as well. I've reordered the last 2 commits so the release commit is the final one to avoid confusion.

@das7pad das7pad assigned briangough and unassigned das7pad Oct 21, 2022
@briangough briangough merged commit 1c23824 into 0.9.x Oct 24, 2022
@briangough briangough deleted the bg-npm-audit branch October 24, 2022 09:04
@briangough
Copy link
Author

For reference, it turns out that socket.io builds the file served to the client at runtime, so this ends up crashing. Discussed with Jakob and decided to move the static file serving into real-time.

@briangough briangough mentioned this pull request Oct 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@das7pad das7pad das7pad approved these changes

Assignees

@briangough briangough

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@briangough @das7pad